Skip to content

feat: add elevation tokens and fix Android dp spec bug#4923

Open
adrcotfas wants to merge 1 commit into@adrcotfas/refactor/tokens_motionfrom
@adrcotfas/refactor/tokens_elevation
Open

feat: add elevation tokens and fix Android dp spec bug#4923
adrcotfas wants to merge 1 commit into@adrcotfas/refactor/tokens_motionfrom
@adrcotfas/refactor/tokens_elevation

Conversation

@adrcotfas
Copy link
Copy Markdown
Collaborator

@adrcotfas adrcotfas commented May 5, 2026 •
edited
Loading

Motivation

Part of the foundation of adding tokens and structure.

Add theme.elevation sys token and clean up elevation types

Replace the ElevationLevels numeric enum with an ElevationLevel string union, add ThemeElevation and theme.elevation.level0-5 as a proper sys token (matching md.sys.elevation.* spec naming), export elevationInputRange as the single source of truth for the [0-5] interpolation range, fix iOS shadow color to read from
theme.colors.shadow instead of a hardcoded #000, and remove ELEVATION_LEVELS_MAP from Menu in favor of the template literal pattern already used in Surface.

Related issue

See https://www.notion.so/callstack/React-Native-Paper-Foundation-for-MD3-Expressive-34c5d027c0f880edba3df107cd35946f?source=copy_link

Merge order:

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 5, 2026

The mobile version of example app from this branch is ready! You can see it here.

This was referenced May 5, 2026
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
Open
satya164
satya164 requested changes May 9, 2026
View reviewed changes
Comment thread src/theme/shadow.tsx
Comment on lines +5 to +6
* @deprecated Pass `theme.colors.shadow` as the second argument and import
* `shadow` from `react-native-paper/theme/tokens/sys/elevation`. Will be removed in a future version.
Copy link
Copy Markdown
Member

@satya164 satya164 May 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users shouldn't be importing things using deep imports. Either make a clean breaking change or keep the API backward-compatible.

Comment thread src/components/Surface.tsx
Comment on lines +99 to +105
inputRange: [...elevationInputRange],
outputRange: [...shadowLayers[layer].height],
}),
},
shadowRadius: elevation.interpolate({
inputRange,
outputRange: iOSShadowOutputRanges[layer].shadowRadius,
inputRange: [...elevationInputRange],
outputRange: [...shadowLayers[layer].shadowRadius],
Copy link
Copy Markdown
Member

@satya164 satya164 May 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are these spread instead of using directly?

Comment thread src/components/Menu/Menu.tsx
backgroundColor:
md3Colors.elevation[ELEVATION_LEVELS_MAP[elevation]],
md3Colors.elevation[
`level${elevation}` as ElevationLevel
Copy link
Copy Markdown
Member

@satya164 satya164 May 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't use as

Comment thread src/components/Surface.tsx
Comment on lines +255 to +256
inputRange: [...elevationInputRange],
outputRange: [...elevationInputRange].map((elevation) => {
Copy link
Copy Markdown
Member

@satya164 satya164 May 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are these spreading?

Comment thread src/theme/tokens/sys/elevation.ts

export const elevationInputRange = [0, 1, 2, 3, 4, 5] as const;

export const androidElevationLevels = [0, 1, 3, 6, 8, 12] as const;
Copy link
Copy Markdown
Member

@satya164 satya164 May 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if these are the actual values, why not expose directly instead of having 0-5 range and translating to a different format? avoiding the indirection would keep things simpler.

Comment thread src/theme/tokens/sys/elevation.ts
Comment on lines +44 to +55
inputRange: [...elevationInputRange],
outputRange: [...shadowLayers[0].height],
}),
},
shadowOpacity: elevation.interpolate({
inputRange: [0, 1],
outputRange: [0, 0.3],
extrapolate: 'clamp',
}),
shadowRadius: elevation.interpolate({
inputRange: [...elevationInputRange],
outputRange: [...shadowLayers[0].shadowRadius],
Copy link
Copy Markdown
Member

@satya164 satya164 May 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the spreading seems unnecessary

Comment thread src/theme/types/elevation.ts
Comment on lines 1 to +9
export type Elevation = 0 | 1 | 2 | 3 | 4 | 5;

export enum ElevationLevels {
'level0',
'level1',
'level2',
'level3',
'level4',
'level5',
}
export type ElevationLevel =
| 'level0'
| 'level1'
| 'level2'
| 'level3'
| 'level4'
| 'level5';
Copy link
Copy Markdown
Member

@satya164 satya164 May 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an opportunity to reduce repetition and simplify these. The same things don't need to be define as enum, union, a numeric union, then later as object in src/theme/tokens/sys/elevation.ts, and a separate range - it could be derived from the map with Object.values, types could be derived from other types, redundant enum could be removed, etc.

Comment thread src/theme/tokens/sys/elevation.ts
elevation: number | Animated.Value = 0,
shadowColor: string
) {
if (elevation instanceof Animated.Value) {
Copy link
Copy Markdown
Member

@satya164 satya164 May 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this work for animated interpolations etc.? i also saw there was a isAnimatedValue helper in another PR

Comment thread src/components/Surface.tsx
Comment on lines +289 to +294
const elevationLevel = [...androidElevationLevels];

const getElevationAndroid = () => {
if (isAnimatedValue(elevation)) {
return elevation.interpolate({
inputRange,
inputRange: [...elevationInputRange],
Copy link
Copy Markdown
Member

@satya164 satya164 May 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is spreading necessary?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@satya164 satya164 satya164 requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@adrcotfas @satya164